Skip to content

Conversation

@gibbz00
Copy link
Contributor

@gibbz00 gibbz00 commented Dec 30, 2025

Fixes:

  • Properly reflect that musl and some descendants do not allow zero-sized payloads being in CMSG_NXTHDR musl_ref.

  • Unsoundness in linux and l4re. Makes sure that the header of next is within max, returning null if not. (Similar to how glibc does it.) It is the fix that should have been made Fix cmsg(3) bugs for musl and OSX #1235, but instead it added pretty severe soundness issues. (Mentioning it for context, not to point fingers.) No checks were previously being done to assert that next as usize + size_of::() < max. Wrapping offset calculations could then lead to buffer over-reads in the following (*next).cmsg_len.

CMSG_NXTHDR Testing:

  • Cleanup correctness. In turn removing some test values being ignored when targeting AIX.
  • Re-enabling Sparc64 testing (fixing CMSG_NXTHDR hits a Bus Error on sparc64-unknown-linux-gnu #1239), which for some reason consistently hit the UB conditions.
  • Actually testing expected behavior when hitting the max controllen boundary.

@rustbot
Copy link
Collaborator

rustbot commented Dec 30, 2025

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in OpenBSD module

cc @semarie

Some changes occurred in the Android module

cc @maurer

…es in CMSG_NXTHDR

musl and its descendants check `next_addr >= max_addr` whilst the rest
do `next_addr > max_addr`. This was previously not reflected in the
implementations, coming to light only after testing was extended to
execute at the controllen boundary.

[musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1
@gibbz00
Copy link
Contributor Author

gibbz00 commented Jan 4, 2026

@rustbot ready

Think that there's only one thread remaining #4903 (comment)

I've added the assertion for you to see how it looks in practice. I can then add a test if you think the check is a good idea 😊 (#4903 (comment))

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new wrapping_add bit LGTM, thanks for all the updates!

@tgross35 tgross35 added this pull request to the merge queue Jan 4, 2026
@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Jan 4, 2026
Merged via the queue into rust-lang:main with commit 17adf2d Jan 4, 2026
51 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
This change makes sure that the header of `next` is within max,
returning null if not. This is similar to how `glibc` does it.

No checks were previously being done to assert that `next as usize +
size_of::<cmsghdr>() < max`. Wrapping offset calculations could then
lead to buffer over-reads in the following `(*next).cmsg_len`.

[glibc ref](https://github.com/bminor/glibc/blob/b71d59074b98ad4abd23c136ec9ad4c26e29ee6d/sysdeps/unix/sysv/linux/cmsg_nxthdr.c#L49-L51)

(backport <rust-lang#4903>)
(cherry picked from commit cdc2077)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
Likely written to make assertions in the unsound CMSG_NXTHDR implementations
introduced in rust-lang#1235. CMSG_NXTHDR(mhdr, current_cmsghdr) should not be concerned
with the value next_cmsghdr.cmsg_len, which the previous implementation did.

(backport <rust-lang#4903>)
(cherry picked from commit 1e43edb)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
Setting `(*pcmsghdr).cmsg_len = cmsg_len as _;` when cmsg_len ranges
from 0 to 64 is invalid as it must always be `>= size_of::<cmsghdr>()`,
rounded up to the nearest alignment boundary.

Some implementations (notably glbic) do check that `cmsg_len >=
size_of::<cmsghdr>()` in `CMSG_NXTHDR`, returning null if so. But
this is more so an extra precaution that is not mentioned in the POSIX
1003.1-2024. It can therefore not be relied on for tests executed on
multiple platforms.

The change also removes the ignoring of some testvalues when targeting AIX.

(backport <rust-lang#4903>)
(cherry picked from commit f391df3)
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Jan 8, 2026
…es in CMSG_NXTHDR

musl and its descendants check `next_addr >= max_addr` whilst the rest
do `next_addr > max_addr`. This was previously not reflected in the
implementations, coming to light only after testing was extended to
execute at the controllen boundary.

[musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1

(backport <rust-lang#4903>)
(cherry picked from commit 17adf2d)
@tgross35 tgross35 mentioned this pull request Jan 8, 2026
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
This change makes sure that the header of `next` is within max,
returning null if not. This is similar to how `glibc` does it.

No checks were previously being done to assert that `next as usize +
size_of::<cmsghdr>() < max`. Wrapping offset calculations could then
lead to buffer over-reads in the following `(*next).cmsg_len`.

[glibc ref](https://github.com/bminor/glibc/blob/b71d59074b98ad4abd23c136ec9ad4c26e29ee6d/sysdeps/unix/sysv/linux/cmsg_nxthdr.c#L49-L51)

(backport <#4903>)
(cherry picked from commit cdc2077)
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
Likely written to make assertions in the unsound CMSG_NXTHDR implementations
introduced in #1235. CMSG_NXTHDR(mhdr, current_cmsghdr) should not be concerned
with the value next_cmsghdr.cmsg_len, which the previous implementation did.

(backport <#4903>)
(cherry picked from commit 1e43edb)
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
Setting `(*pcmsghdr).cmsg_len = cmsg_len as _;` when cmsg_len ranges
from 0 to 64 is invalid as it must always be `>= size_of::<cmsghdr>()`,
rounded up to the nearest alignment boundary.

Some implementations (notably glbic) do check that `cmsg_len >=
size_of::<cmsghdr>()` in `CMSG_NXTHDR`, returning null if so. But
this is more so an extra precaution that is not mentioned in the POSIX
1003.1-2024. It can therefore not be relied on for tests executed on
multiple platforms.

The change also removes the ignoring of some testvalues when targeting AIX.

(backport <#4903>)
(cherry picked from commit f391df3)
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
(backport <#4903>)
(cherry picked from commit 99280f2)
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
(backport <#4903>)
(cherry picked from commit befc34b)
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
…es in CMSG_NXTHDR

musl and its descendants check `next_addr >= max_addr` whilst the rest
do `next_addr > max_addr`. This was previously not reflected in the
implementations, coming to light only after testing was extended to
execute at the controllen boundary.

[musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1

(backport <#4903>)
(cherry picked from commit 17adf2d)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants